Feat: General agent creation, support for app, workflow and slides/docs creation #224
Conversation
- Split monolithic simpleCodeGeneratorAgent into baseAgentBehavior and PhasicAgentBehavior - CodeGeneratorAgent now inherits Agent class (DO) and uses behavior - AgentBehavior relies on AgentInfrastructure (provided by DO) to work, allows to write non-DO coding agents (like cli based) - Phasic for legacy state machine app builder, Agentic for agent driven loop - Agentic would support multiple project types - 'app', 'workflow', 'slides/docs'
…ic coding agent implemented - Abstracted behaviors and objectives - Behavior and Objectives are bot h AgentComponent - CodeGeneratorAgent (Agent DO) houses common business logic - Implemented agentic coding agent and and assistant
- Implemented AI-powered project type prediction (app/workflow/presentation) with confidence scoring and auto-detection when projectType is 'auto' - Enhanced template selection to filter by project type and skip AI selection for single-template scenarios in workflow/presentation types - Added GitHub token caching in CodeGeneratorAgent for persistent OAuth sessions across exports - Updated commitlint config to allow longer commit messages (
- Initialize template cache during agent setup to avoid redundant fetches - Remove redundant project name prompt from template selection - Clean up default projectType fallback logic
- Added concurrency control to prevent duplicate workflow runs on the same PR - Replaced Claude-based comment cleanup with direct GitHub API deletion for better reliability - Enhanced code debugger instructions to handle Vite dev server restarts and config file restrictions
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
worker/agents/core/stateMigration.ts
Outdated
| static migrateIfNeeded(state: CodeGenState, logger: StructuredLogger): CodeGenState | null { | ||
| static migrateIfNeeded(state: AgentState, logger: StructuredLogger): AgentState | null { | ||
| let needsMigration = false; | ||
| const legacyState = state as unknown as Record<string, unknown>; |
There was a problem hiding this comment.
MEDIUM: Overly Permissive Type Casting
While migration code often needs to work with unknown schemas, casting the entire state to Record<string, unknown> reduces type safety throughout the migration function.
Recommendation: Use type guards or more specific type narrowing:
const hasField = (key: string): key is keyof AgentState => key in state;
if (hasField('latestScreenshot')) {
// TypeScript knows the field exists
}This is acceptable for migration code, but consider adding runtime validation using Zod schemas to catch unexpected state shapes early.
| throw new Error(`State behaviorType mismatch: expected ${behaviorType}, got ${this.state.behaviorType}`); | ||
| } | ||
|
|
||
| if (behaviorType === 'phasic') { |
There was a problem hiding this comment.
MEDIUM: Behavior Mismatch Security Risk
This check throws an error if there's a mismatch between expected and actual behavior types, which is good. However, this happens AFTER the behavior instance is created (line 125-128), which could lead to inconsistent state.
Recommendation: Move this validation BEFORE instantiating the behavior:
const isInitialized = Boolean(this.state.query);
const behaviorType = isInitialized
? this.state.behaviorType
: props.behaviorType ?? this.state.behaviorType ?? 'phasic';
// Validate FIRST
if (isInitialized && this.state.behaviorType !== behaviorType) {
throw new Error(`State behaviorType mismatch: expected ${behaviorType}, got ${this.state.behaviorType}`);
}
// Then create behavior with validated type
if (behaviorType === 'phasic') {
this.behavior = new PhasicCodingBehavior(...);
} else {
this.behavior = new AgenticCodingBehavior(...);
}There was a problem hiding this comment.
Seems like a hallucination because we are already doing exactly this
- Replaced unsafe type assertions with proper type guards for legacy state detection - Added explicit type definitions for deprecated state fields and legacy file formats - Eliminated all 'any' types while maintaining backward compatibility with legacy states
|
|
||
| delete (migratedInferenceContext as any).userApiKeys; | ||
| // Remove the deprecated field using type assertion | ||
| const contextWithLegacyField = migratedInferenceContext as unknown as Record<string, unknown>; |
There was a problem hiding this comment.
MEDIUM: Unsafe Type Casting
This type assertion as Record<string, unknown> bypasses TypeScript's type checking. While somewhat acceptable in migration code, it could mask issues.
Recommendation: Add runtime validation to ensure the field actually exists before deletion:
if (migratedInferenceContext && 'userApiKeys' in migratedInferenceContext) {
const { userApiKeys, ...rest } = migratedInferenceContext as unknown as { userApiKeys?: unknown } & InferenceContext;
migratedInferenceContext = rest;
needsMigration = true;
}This approach is safer and more explicit about what's being removed.
| const projectType = props?.projectType ?? this.state.projectType ?? 'app'; | ||
|
|
||
| if (behaviorType === 'phasic') { | ||
| this.behavior = new PhasicCodingBehavior(this as AgentInfrastructure<PhasicState>, projectType); |
There was a problem hiding this comment.
LOW: Type Assertion Without Validation
Casting to AgentInfrastructure<PhasicState> without runtime validation that the state is actually a PhasicState could lead to type mismatches if the behaviorType doesn't align with the state structure.
Recommendation: Add runtime validation:
if (behaviorType === 'phasic') {
// Validate state structure matches PhasicState
if (!('generatedPhases' in this.state) || !('currentDevState' in this.state)) {
throw new Error('State structure does not match phasic behavior requirements');
}
this.behavior = new PhasicCodingBehavior(this as AgentInfrastructure<PhasicState>, projectType);
} else {
// Similar validation for agentic
this.behavior = new AgenticCodingBehavior(this as AgentInfrastructure<AgenticState>, projectType);
}This ensures type safety at runtime, not just compile time.
Code & Security ReviewRecommendation: COMMENT (Minor improvements suggested, no blocking issues) Executive SummaryThis is a well-executed architectural refactoring that successfully modularizes a 2700+ line monolithic agent class into a clean behavior/objective pattern. The architecture is sound and follows good separation of concerns principles. However, there are several type safety violations and a few implementation concerns that should be addressed. Overall Assessment: The refactoring achieves its goals and maintains backward compatibility through state migration. The code is production-ready with minor improvements needed for type safety compliance. Code Quality IssuesType Safety ViolationsMEDIUM - Multiple The codebase violates CLAUDE.md's strict rule: "NEVER use any type". Found in state migration logic: const getTimestamp = (msg: any) => {Impact: Bypasses TypeScript's type checking in migration logic MEDIUM - Unsafe Type Casting in Migration const contextWithLegacyField = migratedInferenceContext as unknown as Record<string, unknown>;
delete contextWithLegacyField.userApiKeys;Impact: Type assertion circumvents compile-time safety LOW - Unvalidated Type Assertion Casting to this.behavior = new PhasicCodingBehavior(this as AgentInfrastructure<PhasicState>, projectType);Impact: Could lead to runtime type mismatches if behaviorType doesn't align with state structure Architecture & DesignPOSITIVE - Excellent Separation of Concerns
POSITIVE - Backward Compatibility
MEDIUM - Complex State Type Hierarchy export type AgentState = PhasicState | AgenticState;Observation: The type guards ( Recommendation: Standardize on type guards throughout the codebase for consistency and safety. DRY Principle CompliancePOSITIVE - Good Code Reuse
LOW - Duplicated Type Guards
Recommendation: Extract to shared utility in Security AnalysisAuthentication & AuthorizationPASS - No Issues Found
Secrets ManagementPASS - Proper Handling
SQL InjectionPASS - No Vulnerabilities
Input ValidationPASS - Adequate Validation
Access ControlPASS - Git Tool Safety
Testing & Validation ConcernsOBSERVATION - Manual Testing Required
RECOMMENDATION: Add integration tests for state migration edge cases Performance ConsiderationsPOSITIVE - Efficient Architecture
LOW - Large Blueprint Context Observation: Acceptable trade-off for autonomous building, but monitor token costs in production. Breaking Changes AssessmentACCURATE - Well Documented
POSITIVE - Backward Compatibility Maintained
Recommendations SummaryMust Fix (Before Merge)
Should Fix (High Priority)
Nice to Have (Low Priority)
ConclusionThis is a high-quality refactoring that achieves its architectural goals. The modular design enables future extensibility (workflows, presentations, CLI agents) while maintaining backward compatibility. The type safety violations are minor and easily fixed. Security practices are sound with no exploitable vulnerabilities found. Approval Status: Approved pending type safety fixes. The violations are isolated to migration code and don't create runtime security risks, but should be corrected to maintain project standards. |
…ess design - Sandbox layer does not rely on templates now, instead expects raw files list - Tools to init/list templates, files - Templates can be chosen by agentic mode after creation - Restructured system prompt with detailed architecture explanations covering virtual filesystem, sandbox environment, and deployment flow - Better tool descriptions - Improved communication guidelines and workflow steps for better agent reasoning and execution
- Replaced agent mode toggle with project mode selector (App/Slides/Chat) that determines behavior type - Implemented agentic behavior detection for static content (docs, markdown) with automatic editor view - Conditionally render PhaseTimeline and deployment controls based on behavior type (phasic vs agentic)
|
|
||
| delete (migratedInferenceContext as any).userApiKeys; | ||
| // Remove the deprecated field using type assertion | ||
| const contextWithLegacyField = migratedInferenceContext as unknown as Record<string, unknown>; |
There was a problem hiding this comment.
MEDIUM: Unsafe Type Casting
Type assertion to Record<string, unknown> circumvents compile-time safety. While acceptable in migration code, safer alternatives exist.
Recommendation:
if (migratedInferenceContext && 'userApiKeys' in migratedInferenceContext) {
// Use destructuring for safer field removal
const { userApiKeys, ...rest } = migratedInferenceContext as InferenceContext & { userApiKeys?: unknown };
migratedInferenceContext = rest;
needsMigration = true;
}This approach is more explicit and type-safe.
| const projectType = props?.projectType ?? this.state.projectType ?? 'app'; | ||
|
|
||
| if (behaviorType === 'phasic') { | ||
| this.behavior = new PhasicCodingBehavior(this as AgentInfrastructure<PhasicState>, projectType); |
There was a problem hiding this comment.
MEDIUM: Unvalidated Type Assertion
Casting to AgentInfrastructure<PhasicState> without runtime validation that the state structure matches could lead to runtime type mismatches.
Recommendation: Add runtime state structure validation:
if (behaviorType === 'phasic') {
// Validate state has phasic-specific fields
if (!('generatedPhases' in this.state) || !('currentDevState' in this.state)) {
throw new Error('State structure does not match phasic behavior requirements');
}
this.behavior = new PhasicCodingBehavior(this as AgentInfrastructure<PhasicState>, projectType);
} else {
// Similar validation for agentic state
this.behavior = new AgenticCodingBehavior(this as AgentInfrastructure<AgenticState>, projectType);
}This ensures the discriminated union's runtime state aligns with the compile-time type.
Code & Security ReviewRecommendation: APPROVE WITH MINOR SUGGESTIONS This is a well-architected refactoring that successfully modularizes a 2700+ line monolithic agent into clean behavior/objective patterns. The code quality is high, security practices are sound, and backward compatibility is properly maintained. Code Quality AssessmentArchitecture - EXCELLENT
Type Safety - 1 Minor ViolationMEDIUM: const getTimestamp = (msg: any) => {This violates CLAUDE.md's strict "NEVER use any type" rule. Fix: const getTimestamp = (msg: ConversationMessage): number => {
if (msg.conversationId && typeof msg.conversationId === 'string' && msg.conversationId.startsWith('conv-')) {
const parts = msg.conversationId.split('-');
if (parts.length >= 2) {
return parseInt(parts[1]) || 0;
}
}
return 0;
};Note: This is existing code that predates this PR. While not introduced by this refactor, it should be fixed to maintain project standards. DRY Compliance - GOOD
Security AnalysisSQL Injection - PASSAll database operations use parameterized queries via template literals. The Cloudflare Agents SDK properly sanitizes these. Authentication & Authorization - PASS
Secrets Management - PASS
Input Validation - PASS
Access Control - PASS
Testing & Migration ConcernsState Migration Logic - SOUND Migration Features:
RecommendationsShould Fix (Post-Merge)
Nice to Have
ConclusionThis is a high-quality architectural refactoring that achieves its goals:
The type safety violation is minor and exists in pre-existing code, not introduced by this PR. Approval Status: ✅ Approved - This PR is ready to merge. |
- Replaced manual template_manager tool with init_suitable_template that uses the original template selector ai - Updated system prompts to emphasize template-first workflow for interactive projects with AI selector as mandatory first step - Simplified template selection process by removing manual list/select commands in favor of intelligent matching ```
- Added conversation history support to AgenticProjectBuilder with message preparation and context tracking - Implemented tool call completion callbacks to sync messages and trigger periodic compactification - Modified AgenticCodingBehavior to queue user inputs during builds and inject them between tool call chains using abort mechanism
- Fix importTemplate to actually work - Fixed template filtering logic to respect 'general' project type - Added behaviorType to logger context for better debugging - fixed not saving behaviorType to state
…ructor - Moved behaviorType and projectType initialization from hardcoded values to constructor-based setup - Changed initial state values to 'unknown' to ensure proper initialization through behavior constructor - Cleared template details cache when importing new templates to prevent stale data
- Moved user input idle check from PhasicCodingBehavior to CodeGeneratorAgent for consistent behavior across all modes - Fixed message order in agenticProjectBuilder to place history after user message instead of before - Added replaceExisting parameter to addConversationMessage for better control over message updates - Enhanced initial state restoration to include queued user messages and behaviorType - Added status and queuePosition fields
- Single convo id needs to be broadcasted but messages need to be saved with unique ids. - Fix message deduplication to use composite key (conversationId + role + tool_call_id) - Improved tool message filtering to validate against parent assistant tool_calls - Removed unused CodingAgentInterface stub file - Simplified addConversationMessage interface by removing replaceExisting parameter
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
1 participant
Summary
This PR refactors the monolithic
SimpleCodeGeneratorAgentinto a modular, extensible agent architecture that separates concerns between "what to build" (objectives) and "how to build it" (behaviors). This enables support for multiple project types beyond web applications, including backend workflows and presentations/slides.Changes
Core Architecture Refactoring
Agent Infrastructure:
AgentInfrastructureinterface (worker/agents/core/AgentCore.ts) - Defines the contract between the Durable Object and agent behaviors/objectivesAgentComponentbase class (worker/agents/core/AgentComponent.ts) - Provides common infrastructure access patterns for all agent componentsCodeGeneratorAgent(worker/agents/core/codingAgent.ts) - Now extendsAgent(DO) and delegates to behavior + objective patternBehavior Pattern (How to Build):
BaseCodingBehavior(worker/agents/core/behaviors/base.ts, 1535 lines) - Abstract base class providing common functionality:PhasicCodingBehavior(worker/agents/core/behaviors/phasic.ts, 852 lines) - Legacy state machine-driven approach for iterative app buildingAgenticCodingBehavior(worker/agents/core/behaviors/agentic.ts, 244 lines) - New agent-driven loop approach for autonomous buildingObjective Pattern (What to Build):
ProjectObjectivebase class (worker/agents/core/objectives/base.ts) - Defines project identity, runtime requirements, and export/deployment logicAppObjective(worker/agents/core/objectives/app.ts) - Full-stack web applications (React + Vite + Workers)WorkflowObjective(worker/agents/core/objectives/workflow.ts) - Backend-only workflows (APIs, scheduled jobs, webhooks)PresentationObjective(worker/agents/core/objectives/presentation.ts) - Slides/presentations (Spectacle library)New Features
Agentic Project Builder:
AgenticProjectBuilder(worker/agents/assistants/agenticProjectBuilder.ts, 411 lines) - Autonomous LLM-driven project builder using tool-calling approachType System:
ProjectType-'app' | 'workflow' | 'presentation'RuntimeType-'sandbox' | 'worker' | 'none'BehaviorType-'phasic' | 'agentic'PhasicState,AgenticState)GenerationContextto support both phasic and agentic contexts with type guardsOperations & Services:
SimpleCodeGenerationOperation(worker/agents/operations/SimpleCodeGeneration.ts) - Lightweight file generation for README and simple tasksICodingAgentinterface - Now behavior-agnostic with consistent method signaturesICodingAgentinterface instead of concrete typesState Management
Enhanced State Schema:
BaseProjectState- Common fields across all behaviorsPhasicState extends BaseProjectState- State machine-specific fields (phases, counter)AgenticState extends BaseProjectState- Agent loop-specific fields (plan)AgentState- Discriminated union ofPhasicState | AgenticStateSchema Updates:
SimpleBlueprintSchema- Base blueprint for all project typesPhasicBlueprintSchema- Extended blueprint for phase-based generationAgenticBlueprintSchema- Simplified blueprint for agent-driven generationFile Changes Summary
Added (13 files):
worker/agents/core/AgentComponent.tsworker/agents/core/AgentCore.tsworker/agents/core/behaviors/agentic.tsworker/agents/core/behaviors/base.tsworker/agents/core/behaviors/phasic.tsworker/agents/core/codingAgent.tsworker/agents/core/objectives/app.tsworker/agents/core/objectives/base.tsworker/agents/core/objectives/presentation.tsworker/agents/core/objectives/workflow.tsworker/agents/assistants/agenticProjectBuilder.tsworker/agents/operations/SimpleCodeGeneration.tsDeleted (3 files):
worker/agents/core/simpleGeneratorAgent.ts(2707 lines) - Split into behaviors and objectivesworker/agents/core/smartGeneratorAgent.ts(40 lines) - Functionality merged into behaviorsworker/agents/operations/ScreenshotAnalysis.ts(134 lines) - Removed unused operationModified (38 files):
ICodingAgentMotivation
Scalability: The monolithic agent class had grown to 2700+ lines and was becoming difficult to maintain and extend.
Extensibility: Adding new project types (workflows, presentations) required duplicating logic or adding more conditionals.
Separation of Concerns: Mixing "what to build" and "how to build it" logic made the code harder to reason about and test.
Future Features: This architecture enables:
Testing
Manual Testing Checklist
Existing Apps (Phasic Behavior):
New Project Types (Agentic Behavior):
Tool Operations:
State Management:
Breaking Changes
Interface Changes:
CodingAgentInterfacerenamed toICodingAgent(follows naming convention)OperationOptions<GenerationContext>instead of concrete agent referenceBackward Compatibility:
behaviorTypeandprojectTypefieldsNotes
This PR description was automatically generated by Claude Code